-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Objective bridges #789
Conversation
bd63ad3
to
20a0265
Compare
ecc3c56
to
b9ca024
Compare
98d020b
to
06ab49a
Compare
So this is for converting unsupported objective types to supported constraints? like quadratic obj to quadratic constraint? max logdet to max of a new variable and a logdet cone constraint? any weird use cases planned? |
Yes, exactly log-det (that is supported by SDPT3 and should be bridged for others) and quadratic objectives which is suppoted by some solvers and should be transformed to RSOC are the main use cases. |
cool. so the objective bridges are aware of the objective sense? like can it check whether the sense is compatible with reformulating using a convex epi/hypograph constraint? |
That's the most tricky part with objective bridges, it would have been easier if both sense and objective are set at the same times. set sense to Min
set function to convex quad
set sense to Max # error
set function to concave quad |
seems like it would make sense to set an objective sense and function at same time. how breaking would it be to change the interface? |
It might be quite breaking. We might start with an error as this use case it not that common and evaluate if it's worth the breakage. |
I'm thinking of cases like https://github.com/JuliaOpt/Pajarito.jl/blob/master/examples/expdesign.jl where you have different convex objectives some of which are "min convex function" and some that are "max concave function" and you want to switch between the objectives easily to see how they affect the solution. Or cases where you try different convex objectives for regression problems, some that are max and some min. |
This would be a usability loss for the API without a clear justification. Why can't we look at the combination of objective sense and objective function and then decide which transformation to apply? |
That's another approach, cache the objective function, in case the bridge is sensitive to min vs max, cache the objective function until |
From the user's point of view, when switching from "min convex" to "max concave", couldn't we clear the objective by switching to feasibility mode as an intermediate step? |
b5e49c7
to
3a22a57
Compare
e7f21a7
to
61b22cc
Compare
1422bc9
to
e47d235
Compare
Adding to the v0.9.2 milestone as currently, bridged variables cannot be used in SingleVariable objective which is a regression as SDOI could handle that. This PR fixes this issues by force-bridging the SingleVariable to a ScalarAffineFunction with Bridges.Objective.FunctionizeBridge |
bf29a46
to
e3f6c93
Compare
Just to say @JiazhengZhu ran into this when trying to test SDPA-GMP with some JuMP examples. |
ecded8e
to
2ec0202
Compare
Codecov Report
@@ Coverage Diff @@
## master #789 +/- ##
==========================================
+ Coverage 94.87% 94.97% +0.09%
==========================================
Files 77 80 +3
Lines 8094 8274 +180
==========================================
+ Hits 7679 7858 +179
- Misses 415 416 +1
Continue to review full report at Codecov.
|
This PR should be ready now. The I have created a PR with an independent subset of the changes for easier review: #867 |
src/Bridges/Objective/bridge.jl
Outdated
"Objective bridge of type `$(typeof(bridge))` does not support" * | ||
" modifying the objective sense. As a workaround, set the sense to" * | ||
" `MOI.FEASIBILITY_SENSE` to clear the objective function and" * | ||
" bridges.")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what is JuMP going to do? Is it possible to easily set FEASIBILITY_SENSE
from JuMP?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The user can do set_objective_sense(model, MOI.FEASIBILITY_SENSE)
. This is not ideal but this is a rare use case. We can implement #789 (comment) if this is an issue
return MOI.supports(b.model, attr) | ||
end | ||
end | ||
struct ObjectiveFunctionValue{F<:MOI.AbstractScalarFunction} end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it necessary to have a different attribute? What's the difference between ObjectiveFunctionValue
and ObjectiveValue
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a docstring
2ec0202
to
aa618fc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess #870 is going to be merged first, and then this rebased on-top?
aa618fc
to
f96a3dc
Compare
4fcf55b
to
ccba971
Compare
Note that the base branch set is
bl/variable_bridge
but I don't plan to merge it into it, it's only set to have a small diff. This PR will only be merged oncebl/variable_bridge
is merged into master.Closes #529